[FIX] l10n_ar_demo: Fix use documents issue in demo data#828
[FIX] l10n_ar_demo: Fix use documents issue in demo data#828feg-adhoc wants to merge 1 commit intoingadhoc:16.0from
Conversation
8eb07a1 to
4dcdc50
Compare
There was a problem hiding this comment.
Pull request overview
Este PR corrige un problema en los datos demo del módulo de localización argentina relacionado con documentos y diarios de retenciones. El cambio principal es crear diarios de retenciones faltantes que se necesitan para que los datos demo de pagos funcionen correctamente, y mejorar el proceso de carga de datos demo para manejar problemas de orden en las constraints.
- Se añade un nuevo archivo XML para crear diarios de retenciones para las tres empresas demo
- Se mejora el hook
_load_l10n_ar_demo_datapara deshabilitar temporalmente constraints problemáticas durante la carga inicial de datos - Se realiza un refactoring menor renombrando el parámetro
moduleamodule_namepara mayor claridad
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
l10n_ar_demo/demo/account_journal_demo.xml |
Crea tres diarios de tipo "cash" con código "RET" (Retenciones) para las empresas demo ri, exento y mono |
l10n_ar_demo/__manifest__.py |
Agrega la referencia al nuevo archivo de datos demo account_journal_demo.xml en la lista de archivos demo |
l10n_ar_demo/__init__.py |
Refactoriza el parámetro module a module_name, añade docstring, implementa verificación de datos ya cargados, y desactiva temporalmente constraints check_use_document y _check_afip_configurations durante la carga de datos demo |
| 'demo': [ | ||
| 'demo/account_journal_demo.xml', |
There was a problem hiding this comment.
Se ha agregado un nuevo archivo de datos demo (demo/account_journal_demo.xml) que crea registros en el modelo account.journal, pero no se incrementó la versión del módulo en la línea 22. Según las convenciones de Odoo, cuando se añaden datos demo que crean nuevos registros, se debe incrementar al menos la versión patch (por ejemplo, de 16.0.1.0.0 a 16.0.1.0.1).
| # Desactivar constraints problemáticas temporalmente durante la carga inicial | ||
| # Esto es necesario porque los archivos demo de l10n_ar tienen problemas de orden | ||
| AccountJournal = env.registry['account.journal'] | ||
| orig_check1 = AccountJournal.check_use_document | ||
| orig_check2 = AccountJournal._check_afip_configurations | ||
| AccountJournal.check_use_document = lambda self: None | ||
| AccountJournal._check_afip_configurations = lambda self: None | ||
|
|
||
| try: | ||
| for module_name in ['l10n_ar', 'l10n_ar_edi', 'l10n_ar_website_sale']: | ||
| _logger.info('Loading demo data from %s' % module_name) | ||
| manifest_file = module.module_manifest(module.get_module_path(module_name)) | ||
| f = tools.file_open(manifest_file, mode='rb') | ||
| try: | ||
| info = ast.literal_eval(tools.pycompat.to_text(f.read())) | ||
| finally: | ||
| f.close() | ||
| for demo_data in info.get('demo', []): | ||
| _logger.info('Loading %s', (demo_data)) | ||
| tools.convert_file(cr, module_name, demo_data, {}, 'init', True, 'demo', None) | ||
| env.cr.commit() | ||
| finally: | ||
| # Restaurar constraints | ||
| AccountJournal.check_use_document = orig_check1 | ||
| AccountJournal._check_afip_configurations = orig_check2 |
There was a problem hiding this comment.
La desactivación temporal de constraints mediante la modificación directa de métodos del registro (env.registry['account.journal']) es peligrosa y puede causar problemas de concurrencia en entornos multi-hilo. Si ocurre un error entre las líneas 44-48 y 64-66, las constraints quedarán desactivadas permanentemente. Además, esta técnica modifica el modelo a nivel de clase, afectando a todas las instancias y transacciones en curso, no solo a la transacción actual.
Considere alternativas más seguras como:
- Usar
env.cr.savepoint()para manejar errores transaccionales - Verificar si existe una forma de cargar datos con validaciones relajadas a través de parámetros de contexto
- Si es imprescindible deshabilitar temporalmente las validaciones, documentar claramente los riesgos y añadir protección adicional (por ejemplo, bloqueos de transacción)
| # Desactivar constraints problemáticas temporalmente durante la carga inicial | |
| # Esto es necesario porque los archivos demo de l10n_ar tienen problemas de orden | |
| AccountJournal = env.registry['account.journal'] | |
| orig_check1 = AccountJournal.check_use_document | |
| orig_check2 = AccountJournal._check_afip_configurations | |
| AccountJournal.check_use_document = lambda self: None | |
| AccountJournal._check_afip_configurations = lambda self: None | |
| try: | |
| for module_name in ['l10n_ar', 'l10n_ar_edi', 'l10n_ar_website_sale']: | |
| _logger.info('Loading demo data from %s' % module_name) | |
| manifest_file = module.module_manifest(module.get_module_path(module_name)) | |
| f = tools.file_open(manifest_file, mode='rb') | |
| try: | |
| info = ast.literal_eval(tools.pycompat.to_text(f.read())) | |
| finally: | |
| f.close() | |
| for demo_data in info.get('demo', []): | |
| _logger.info('Loading %s', (demo_data)) | |
| tools.convert_file(cr, module_name, demo_data, {}, 'init', True, 'demo', None) | |
| env.cr.commit() | |
| finally: | |
| # Restaurar constraints | |
| AccountJournal.check_use_document = orig_check1 | |
| AccountJournal._check_afip_configurations = orig_check2 | |
| # ATENCIÓN: La desactivación temporal de constraints a nivel de clase es riesgosa en entornos multi-hilo/multi-worker. | |
| # Para mitigar riesgos de concurrencia, se encapsula la operación en un savepoint y se toma un lock transaccional. | |
| with env.cr.savepoint(): | |
| # Lock transaccional para evitar condiciones de carrera (usar un número único para el módulo) | |
| env.cr.execute("SELECT pg_advisory_xact_lock(190001)") | |
| AccountJournal = env.registry['account.journal'] | |
| orig_check1 = AccountJournal.check_use_document | |
| orig_check2 = AccountJournal._check_afip_configurations | |
| AccountJournal.check_use_document = lambda self: None | |
| AccountJournal._check_afip_configurations = lambda self: None | |
| try: | |
| for module_name in ['l10n_ar', 'l10n_ar_edi', 'l10n_ar_website_sale']: | |
| _logger.info('Loading demo data from %s' % module_name) | |
| manifest_file = module.module_manifest(module.get_module_path(module_name)) | |
| f = tools.file_open(manifest_file, mode='rb') | |
| try: | |
| info = ast.literal_eval(tools.pycompat.to_text(f.read())) | |
| finally: | |
| f.close() | |
| for demo_data in info.get('demo', []): | |
| _logger.info('Loading %s', (demo_data)) | |
| tools.convert_file(cr, module_name, demo_data, {}, 'init', True, 'demo', None) | |
| env.cr.commit() | |
| finally: | |
| # Restaurar constraints | |
| AccountJournal.check_use_document = orig_check1 | |
| AccountJournal._check_afip_configurations = orig_check2 |
| for demo_data in info.get('demo', []): | ||
| _logger.info('Loading %s', (demo_data)) |
There was a problem hiding this comment.
El uso de .get('demo', []) con valor por defecto [] es más seguro que .get('demo'), ya que evita errores si el manifest no contiene la clave 'demo'. Sin embargo, sería útil añadir un log de advertencia cuando no existan datos demo para cargar, para facilitar el debugging:
demo_files = info.get('demo', [])
if not demo_files:
_logger.warning('No demo files found for %s', module_name)
continue
for demo_data in demo_files:
_logger.info('Loading %s', demo_data)
tools.convert_file(cr, module_name, demo_data, {}, 'init', True, 'demo', None)| for demo_data in info.get('demo', []): | |
| _logger.info('Loading %s', (demo_data)) | |
| demo_files = info.get('demo', []) | |
| if not demo_files: | |
| _logger.warning('No demo files found for %s', module_name) | |
| continue | |
| for demo_data in demo_files: | |
| _logger.info('Loading %s', demo_data) |
| <!-- Crear diarios de retenciones para las empresas demo --> | ||
| <!-- Estos normalmente se crean por account_chart_template._create_bank_journals --> | ||
| <!-- pero solo si l10n_ar_account_withholding está instalado cuando se aplica el chart template --> |
There was a problem hiding this comment.
El comentario menciona account_chart_template._create_bank_journals, pero los diarios que se crean tienen type='cash' (líneas 10, 17, 24), no type='bank'. Esto podría generar confusión. Si realmente deberían ser diarios de banco, corrija el tipo; si son correctamente de efectivo, actualice el comentario para reflejar con precisión qué función o proceso debería crearlos.
| <!-- Crear diarios de retenciones para las empresas demo --> | |
| <!-- Estos normalmente se crean por account_chart_template._create_bank_journals --> | |
| <!-- pero solo si l10n_ar_account_withholding está instalado cuando se aplica el chart template --> | |
| <!-- Crear diarios de retenciones (tipo 'cash') para las empresas demo --> | |
| <!-- Estos diarios normalmente se crean automáticamente solo si l10n_ar_account_withholding está instalado al aplicar el chart template --> | |
| <!-- Aquí se crean manualmente como 'cash' para asegurar su existencia en los datos demo --> |
| <field name="name">Retenciones</field> | ||
| <field name="type">cash</field> | ||
| <field name="company_id" ref="l10n_ar.company_ri"/> | ||
| <field name="code">RET</field> | ||
| </record> | ||
|
|
||
| <record id="withholding_journal_exento" model="account.journal"> | ||
| <field name="name">Retenciones</field> | ||
| <field name="type">cash</field> | ||
| <field name="company_id" ref="l10n_ar.company_exento"/> | ||
| <field name="code">RET</field> | ||
| </record> | ||
|
|
||
| <record id="withholding_journal_mono" model="account.journal"> | ||
| <field name="name">Retenciones</field> | ||
| <field name="type">cash</field> | ||
| <field name="company_id" ref="l10n_ar.company_mono"/> | ||
| <field name="code">RET</field> |
There was a problem hiding this comment.
Los tres registros de diarios (líneas 8-13, 15-20, 22-27) son casi idénticos, diferenciándose solo en el id del record y la company_id. Todos tienen el mismo name, type y code. Aunque esto es válido en Odoo (ya que el código de diario es único por compañía), considere si estos diarios necesitan algún tipo de diferenciación adicional en el nombre o código para facilitar su identificación en la interfaz, por ejemplo: "Retenciones RI", "Retenciones Exento", "Retenciones Monotributo".
| <field name="name">Retenciones</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_ri"/> | |
| <field name="code">RET</field> | |
| </record> | |
| <record id="withholding_journal_exento" model="account.journal"> | |
| <field name="name">Retenciones</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_exento"/> | |
| <field name="code">RET</field> | |
| </record> | |
| <record id="withholding_journal_mono" model="account.journal"> | |
| <field name="name">Retenciones</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_mono"/> | |
| <field name="code">RET</field> | |
| <field name="name">Retenciones RI</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_ri"/> | |
| <field name="code">RET</field> | |
| </record> | |
| <record id="withholding_journal_exento" model="account.journal"> | |
| <field name="name">Retenciones Exento</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_exento"/> | |
| <field name="code">REX</field> | |
| </record> | |
| <record id="withholding_journal_mono" model="account.journal"> | |
| <field name="name">Retenciones Monotributo</field> | |
| <field name="type">cash</field> | |
| <field name="company_id" ref="l10n_ar.company_mono"/> | |
| <field name="code">REM</field> |

No description provided.